Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiplayer API now respects allow_object_decoding #27398

Closed
wants to merge 1 commit into from

Conversation

Faless
Copy link
Collaborator

@Faless Faless commented Mar 25, 2019

Note that var decoding is still allowed for StreamPeer.get_var.
I think we need to add an object_decoding property to StreamPeer too (which could be probably cherry picked to 3.0.).
2.1 is not affected by any of this as object decoding is not implemented.

I'm also making a separate branch to fix in 3.0 (as the bug is there, but everything related to High Level multiplayer is in SceneTree so cherry picking is not an option).

Fixes #27395 .

I'm also wondering, if we should change the defaults for decode_variant and encode_variant to be safe (no object decoding), and update all their usage where we want object encoding/decoding with the extra parameter to true.

@akien-mga
Copy link
Member

Note that var decoding is still allowed for StreamPeer.get_var.
I think we need to add an object_decoding property to StreamPeer too (which could be probably cherry picked to 3.0.).

That sounds good to me.

I'm also wondering, if we should change the defaults for decode_variant and encode_variant to be safe (no object decoding), and update all their usage where we want object encoding/decoding with the extra parameter to true.

I agree, it's better that the defaults are safe. We can't expect people to read the documentation to ensure that their use of these APIs is safe, so it's better that they have to read them to find out how to use the unsafe behaviour when needed :)

It's a slight compat breakage but that's justified for security purposes IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RPC passes Objects by default
2 participants